Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔨 render thumbnails in CF workers using chart configs from R2 #3847

Closed

Conversation

danyx23
Copy link
Contributor

@danyx23 danyx23 commented Aug 5, 2024

This PR changes how the CF thumnail worker gets the config for a chart. It used to be the case that it would fetch the HTML file of the grapher page at the given slug and extract the config from that HTML. Now it looks up the grapher config json file in an R2 bucket.

This PR only changes this for published charts accessed by slug. A later PR will enable this also by UUID.

Cloudflare is a bit weird with the intersection of support of various features between CF workers/pages functions, R2 and local/remote dev support:

  • Initially I wanted to use bindings in CF functions. For local development, wrangler dev does not support using a remote R2 bucket which is annoying
  • I then thought I'd switch to fetching data from R2 buckets via HTTPS instead of bindings, similar to what we do with our variable data/metadata.json files. I realized then though that this only works if the bucket itself is accessible publicly via HTTPS. I'd like to keep the R2 bucket internal and not expose it directly.
  • My next approach was to try to use the S3 API in the CF pages function. This didn't work because the default AWS library doesn't work in CF functions and the simpler replacement libraries that I could find were very barebones and made you deal with XML manually
  • So finally I decided to go back to bindings and accepting the somewhat annoying dev story

Copy link
Contributor Author

danyx23 commented Aug 5, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @danyx23 and the rest of your teammates on Graphite Graphite

@danyx23 danyx23 force-pushed the grapher-configs-in-r2 branch from e9eace9 to 9105d37 Compare August 6, 2024 14:34
@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch 2 times, most recently from 5819ecd to 6e9dc2f Compare August 6, 2024 18:28
@owidbot
Copy link
Contributor

owidbot commented Aug 6, 2024

Quick links (staging server):

Site Admin Wizard

Login: ssh owid@staging-site-use-r2-grapher-configs-in-cf-functions

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2024-08-06 18:38:04 UTC
Execution time: 1.25 seconds

@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch 2 times, most recently from 7c43c3d to ae06e0b Compare August 7, 2024 09:47
@danyx23 danyx23 force-pushed the grapher-configs-in-r2 branch from 7c77d87 to ff63d5d Compare August 7, 2024 20:36
@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch 5 times, most recently from 6478da8 to 76d65b2 Compare August 8, 2024 14:23
@danyx23 danyx23 changed the title 🔨 configure r2 bindings 🔨 render thumbnails in CF workers using chart configs from R2 Aug 8, 2024
@danyx23 danyx23 force-pushed the grapher-configs-in-r2 branch from 799a43f to 5609c8c Compare August 9, 2024 14:48
@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch from 76d65b2 to ef42cf3 Compare August 9, 2024 14:48
@danyx23 danyx23 force-pushed the grapher-configs-in-r2 branch from 5609c8c to 6c1fec3 Compare August 9, 2024 15:06
@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch from ef42cf3 to d033124 Compare August 9, 2024 15:07
@danyx23 danyx23 force-pushed the grapher-configs-in-r2 branch from 6c1fec3 to 36d9326 Compare August 12, 2024 09:40
@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch from d033124 to 69fe865 Compare August 12, 2024 09:40
@danyx23 danyx23 force-pushed the grapher-configs-in-r2 branch from 36d9326 to f704dff Compare August 12, 2024 11:35
@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch from 69fe865 to f8b200f Compare August 12, 2024 11:35
@danyx23 danyx23 force-pushed the use-r2-grapher-configs-in-cf-functions branch from f8b200f to 87f0855 Compare August 13, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants